awesome. please merge this :) 👍 thanks @rstacruz.

@a8m That's pretty sweet!

danielstjules:~/git/mocha (master =)
$ cat example.js
it('test', function(done) {
  done(new Error('Something went wrong!'));
});

danielstjules:~/git/mocha (master =)
$ ./bin/mocha example.js


  ․

  0 passing (4ms)
  1 failing

  1)  test:
     Error: Something went wrong!
      at Context.<anonymous> (/Users/danielstjules/git/mocha/example.js:2:8)
      at Test.Runnable.run (/Users/danielstjules/git/mocha/lib/runnable.js:233:15)
      at Runner.runTest (/Users/danielstjules/git/mocha/lib/runner.js:387:10)
      at /Users/danielstjules/git/mocha/lib/runner.js:470:12
      at next (/Users/danielstjules/git/mocha/lib/runner.js:312:14)
      at /Users/danielstjules/git/mocha/lib/runner.js:322:7
      at next (/Users/danielstjules/git/mocha/lib/runner.js:257:23)
      at Immediate._onImmediate (/Users/danielstjules/git/mocha/lib/runner.js:289:5)
      at processImmediate [as _immediateCallback] (timers.js:358:17)



danielstjules:~/git/mocha (master =)
$ git checkout stack-trace
Switched to branch 'stack-trace'
danielstjules:~/git/mocha (stack-trace)
$ ./bin/mocha example.js


  ․

  0 passing (7ms)
  1 failing

  1)  test:
     Error: Something went wrong!
      at Context.<anonymous> (example.js:2:8)
      at Test.Runnable.run (lib/runnable.js:229:15)
      at Runner.runTest (lib/runner.js:390:10)
      at lib/runner.js:473:12
      at next (lib/runner.js:315:14)
      at lib/runner.js:325:7
      at next (lib/runner.js:260:23)
      at Immediate._onImmediate (lib/runner.js:292:5)

In addition to the work here, I wonder if there'd be interest in removing parts of the trace that propagated from mocha itself. 75% of the calls in that trace are from runner.js, which isn't something I think we need to see? For example, with the test case above, I think the following output would be a lot nicer:

  1)  test:
     Error: Something went wrong!
      at Context.<anonymous> (example.js:2:8)

You'd just have to traverse the trace, bottom up, in search of the first line that wasn't a file in mocha. Once found, just drop the lines below. This way you're not hiding any information related to the user's code. I remember doing something similar with https://github.com/danielstjules/pho a while back.

@danielstjules you get this trace because you run it from mocha itself (it's assume that is your code, and you care of it).
PTAL on the mocha filter.

Exactly what I hoped for. :) +1

$ mocha example.js

  ․

  0 passing (5ms)
  1 failing

  1)  test:
     Error: Something went wrong!
      at Context.<anonymous> (example.js:2:8)

If I may just chime in with API design input, perhaps --stack-trace is not the best choice for a name. It seems to imply that stack traces will not be shown unless it's turned on. Maybe it would be better as --full-stack-trace.

Good work!

Good point @rstacruz,
--full-stack-trace is too long imo, maybe --full-trace could be better ?.

update: renaming flag to: --full-trace

Would you be willing to run git reset HEAD~ and git add -p to keep your feature-related changes in a separate commit from style fixes? It'll keep blame a lot cleaner. And makes the PR quicker to review. I think @rstacruz's comment on if (nodeVersion < 0x00090B) { is an example of not being sure at a glance what's feature-related vs clean up.

sure @danielstjules

ping @danielstjules

Sorry to nitpick, for some reason a8m@8d90e59 still contains changes to extraGlobals, along with a bunch of other (and totally valid) style changes.

Would you mind force-pushing https://github.com/danielstjules/mocha/commits/stack-trace onto your branch? I cleaned up that second commit a bit (went from 400 line changes, to 270)

PTAL @danielstjules

LGTM :) +1

cc @travisjeffery @boneskull @dasilvacontin

@travisjeffery can you please take a look ?

i tried this and it didn't work for me. lemme try again...

ah nvm there we go, yup lgtm

That's awesome, thx @travisjeffery

Is this something that will land in 2.3.0?

I'll update mocha-clean (the module this was based off of) to throw a warning for newer versions of mocha.

@rstacruz supposedly. I'm very excited to see this published soon. 😄

👍

mocha-clean/brief is so much more readable!

This is actually super UNhelpful in certain cases (mainly async things); I'm running Mocha programmatically and can't seem to work out how to disable this. Specifying fullTrace: true or fullStackTrace: true to the Mocha constructor or calling mocha.fullTrace() don't seem to work. Halp?

Seems to be using this here: https://github.com/mochajs/mocha/blob/master/lib/runner.js#L209 -- not sure why, since the this object does not contain that option when it is enabled on mocha.

Seems to be using this here: https://github.com/mochajs/mocha/blob/master/lib/runner.js#L209 -- not sure why, since the this object does not contain that option when it is enabled on mocha.

It's set here: https://github.com/mochajs/mocha/blob/master/lib/mocha.js#L429

fullStackTrace: true or mocha.fullTrace() should do the trick. Else, something is going wrong.

Indeed, something is going wrong. It doesn't do the trick.

Can you share a Gist with sample code that reproduces the issue?

Not easily, but it may be indirectly my fault; I'll have to dig deeper. I don't see why the 'this' argument would be wrong and mocha still run at all, but I'll find out. It seemed such an obvious disconnect, that I figured at first that whoever submitted the patch just didn't test the opposite case thoroughly and made a logic error.

Yep, I got it sorted. It's weird to set the options and then just copy them over, seems very non-DRY. I had to monkey patch mocha.run to emit an extra event; looks like there's support in the code that will let me not have to do that anymore now. Sorry for the confusion and thanks for the prompt response.

In the opposite direction of my original comment, internal node filtering doesn't catch _stream_readable.js or net.js among other things; not sure if that is intentional. I'm pretty sure that all internal modules do not have paths, so that function could probably be simplified and made more forward-compatible too.